Skip to content

(feat): use np.zeros for buffer creation with fill_value=0 #3082

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
May 23, 2025
Merged

(feat): use np.zeros for buffer creation with fill_value=0 #3082

merged 3 commits into from
May 23, 2025

Conversation

@ilan-gold
Copy link
Contributor

@ilan-gold ilan-gold commented May 22, 2025 •
edited
Loading

I was profiling code and noticed a non-trivial amount of time spent on np.full with a fill_value=0 so I did a little digging and at least on my machine:

In [1]: import numpy as np

In [2]: %timeit np.full((10_000, 10_000), dtype=np.float32, fill_value=0, order="C")
33.2 ms ± 784 μs per loop (mean ± std. dev. of 7 runs, 10 loops each)

In [3]: %timeit np.zeros((10_000, 10_000), dtype=np.float32, order="C")
1.04 μs ± 1.48 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)

For example:

zarr-python/src/zarr/core/array.py

Lines 1285 to 1290 in 481550a

out_buffer = prototype.nd_buffer.create(
shape=indexer.shape,
dtype=out_dtype,
order=self.order,
fill_value=self.metadata.fill_value,
)

and

zarr-python/src/zarr/codecs/sharding.py

Lines 441 to 443 in 481550a

out = chunk_spec.prototype.nd_buffer.create(
shape=shard_shape, dtype=shard_spec.dtype, order=shard_spec.order, fill_value=0
)

are both points where this happens

TODO:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/user-guide/*.rst
  • Changes documented as a new file in changes/
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

@github-actions github-actions bot added the needs release notes Automatically applied to PRs which haven't added release notes label May 22, 2025
@ilan-gold ilan-gold changed the title (feat): use np.zeros for buffer creation fill_value=0 (feat): use np.zeros for buffer creation with fill_value=0 May 22, 2025
@d-v-b
Copy link
Contributor

d-v-b commented May 22, 2025

out of curiosity, is there any performance difference between np.zeros(...)[:] = fill_value and np.full(..., fill_value)? I would expect not but I have no idea.

d-v-b
d-v-b reviewed May 22, 2025
View reviewed changes
@d-v-b d-v-b added the performance Potential issues with Zarr performance (I/O, memory, etc.) label May 22, 2025
@ilan-gold
Copy link
Contributor Author

Not much :/

In [1]: import numpy as np
In [2]: %timeit d = np.zeros((10_000, 10_000), dtype=np.float32, order="C"); d[:] = 1
35.1 ms ± 934 μs per loop (mean ± std. dev. of 7 runs, 10 loops each)
In [3]: %timeit np.full((10_000, 10_000), dtype=np.float32, fill_value=1, order="C")
37.7 ms ± 2.15 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)

@ilan-gold
Copy link
Contributor Author

@d-v-b I think the failing test is unrelated to this PR. I'm also seeing it locally on main

@github-actions github-actions bot removed the needs release notes Automatically applied to PRs which haven't added release notes label May 22, 2025
@dstansby dstansby merged commit f674236 into zarr-developers:main May 23, 2025
30 checks passed
@ilan-gold ilan-gold deleted the ig/fill_value_0 branch May 23, 2025 11:07
@d-v-b d-v-b added this to the 3.0.9 milestone Jun 30, 2025
@d-v-b
Copy link
Contributor

d-v-b commented Jun 30, 2025

@meeseeksdev backport to 3.0.9

@lumberbot-app
Copy link

lumberbot-app bot commented Jun 30, 2025

Can't Dooooo.... It seem like this is already backported (commit is empty).I won't do anything. MrMeeseeks out.

@d-v-b d-v-b removed this from the 3.0.9 milestone Jun 30, 2025
@ilan-gold ilan-gold mentioned this pull request Aug 5, 2025
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@d-v-b d-v-b d-v-b left review comments

+1 more reviewer

@dstansby dstansby dstansby approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

performance Potential issues with Zarr performance (I/O, memory, etc.)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ilan-gold @d-v-b @dstansby